Skip to content

Support inline JavaScript events (v2) #1290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 25 commits into from
Apr 19, 2025
Merged

Support inline JavaScript events (v2) #1290

merged 25 commits into from
Apr 19, 2025
+275 −43

Conversation

shawncrawley
Copy link
Contributor

@shawncrawley shawncrawley commented Apr 1, 2025 •
edited
Loading

Description

This is an alternative solution to PR #1289, so only one of them need be merged. I created this separately to better compare approaches. This approach modifies the VDOM schema to include a new jsExecutables property alongside eventHandlers.

Checklist

Please update this checklist as you complete each item:

  • Tests have been developed for bug fixes or new functionality.
  • The changelog has been updated, if necessary.
  • Documentation has been updated, if necessary.
  • GitHub Issues closed by this PR have been linked.

By submitting this pull request I agree that all contributions comply with this project's open source license(s).

@shawncrawley shawncrawley marked this pull request as ready for review April 1, 2025 16:22
@shawncrawley
Copy link
Contributor Author

This is ready to be looked at - again, aside from the changelog.

I can get this to work "out of the box" with string_to_reactpy if I simply loosen the "on" event listener regex to not expect camelCase, which is usually how raw HTML will be written. The question is if we should coerce all properties starting with "on" to camelCase on the Python/Vdom side (e.g. "onclick" to "onClick"). Right now I'm not doing that, which is how I've written the string_to_reactpy test to expect the Vdom result.

I ultimately assumed that coercing them to camelCase on the Vdom side is unnecessary overhead. Let me know if you agree.

@Archmonger
Copy link
Contributor

onclick should already be automatically mutated into onClick by string_to_reactpy at this LOC.

Archmonger
Archmonger requested changes Apr 2, 2025
View reviewed changes
@shawncrawley
Copy link
Contributor Author

All comments have been addressed, changelog is updated... This PR should be ready to go.

Archmonger
Archmonger requested changes Apr 4, 2025
View reviewed changes
Copy link
Contributor

@Archmonger Archmonger left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be merged after handling a few more nitpicks

@Archmonger Archmonger mentioned this pull request Apr 4, 2025
Closed
4 tasks
Archmonger
Archmonger reviewed Apr 4, 2025
View reviewed changes
@Archmonger Archmonger changed the title Support raw JavaScript events - v2, w/VDOM schema change approach Support inline JavaScript event (v2) Apr 4, 2025
@Archmonger Archmonger changed the title Support inline JavaScript event (v2) Support inline JavaScript events (v2) Apr 4, 2025
@Archmonger Archmonger merged commit bf06b60 into reactive-python:develop Apr 19, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Archmonger Archmonger Archmonger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shawncrawley @Archmonger